fix(ssh): survive csh/tcsh login shells when running multiline relay commands#8714
Open
xianjianlf2 wants to merge 1 commit into
Open
fix(ssh): survive csh/tcsh login shells when running multiline relay commands#8714xianjianlf2 wants to merge 1 commit into
xianjianlf2 wants to merge 1 commit into
Conversation
…commands sshd hands every exec command string to the user's login shell. csh/tcsh cannot keep a single-quoted multiline argument intact — each line is re-parsed as its own csh command — so Orca's multiline `exec /bin/sh -c '<script>'` wrapper never reached /bin/sh on csh hosts. Node detection always failed with "Node.js not found on remote host" even with Node 18+ installed, and every other multiline relay command broke the same way. Collapse multiline commands to one line in wrapRemoteCommandForPosixShell: encode backslashes and newlines, and let /bin/sh rebuild the script via `eval "$(printf %b ...)"` — printf is a POSIX sh builtin, eval leaves stdin free for streaming commands, and exit codes propagate. Single-line commands keep the exact previous form. Also fix the login-shell fallback probe: csh/tcsh reject the combined `-lc` flag and have no `command` builtin, so probe them with `-c 'which node'` (non-login csh still reads .cshrc, where EDA/HPC farms set PATH). Fixes stablyai#8701 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughRemote command wrapping now preserves direct handling for single-line commands and encodes multiline scripts into newline-free 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 641a4c8c-9917-4730-8f79-a58087b64f43
📒 Files selected for processing (4)
src/main/ssh/ssh-connection-utils.tssrc/main/ssh/ssh-remote-command-wrapping.test.tssrc/main/ssh/ssh-remote-node-resolution.test.tssrc/main/ssh/ssh-remote-node-resolution.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #8701
Problem
Adding an SSH remote whose login shell is
csh/tcsh(common on EDA/HPC farms, often centrally managed) fails withNode.js not found on remote host, even with Node 18+ installed and on PATH.Root cause
sshd hands every exec command string to the user's login shell for parsing. Orca wraps remote commands as
exec /bin/sh -c '<script>'(wrapRemoteCommandForPosixShell), which is the right idea — but csh/tcsh cannot keep a single-quoted multiline argument intact. Each line is re-parsed as its own csh command (Unmatched '''./for: Command not found.), so/bin/shnever runs. Both Node-detection strategies were multiline, and the wrapper is used for all relay commands, so any multiline relay command broke the same way. Reproduced locally:/bin/csh -c "<wrapped multiline>"fails exactly as the issue reports; the single-line form succeeds.A second, independent break: the login-shell fallback ran
"$SHELL" -lc 'command -v node'. csh/tcsh reject the combined-lcflag outright (Unknown option: '-lc'), and have nocommandbuiltin either.Fix
wrapRemoteCommandForPosixShell— when the command contains a newline, collapse it to one line: encode backslashes and newlines, and let/bin/shrebuild the script witheval "$(printf %b '<encoded>')".printfis a POSIX sh builtin (nobase64dependency),evalkeeps stdin free for commands that stream data (file transfer), and exit codes propagate. Single-line commands keep the exact previous form, so existing behavior on POSIX/fish/nushell login shells is untouched.-c 'which node'instead of-lc 'command -v node':whichis the csh-family PATH resolver, and non-login csh still reads.cshrc, which is where EDA/HPC farms set PATH.Testing
ssh-remote-command-wrapping.test.tsruns the wrapped output through real login shells (sh,bash,zsh,dash,csh,tcsh— skipping ones not installed), exactly how sshd invokes them: multiline scripts with quotes/backslashes/expansions, exit-code propagation, and stdin passthrough. The csh/tcsh cases fail on the old wrapper and pass with the fix.-c 'which node'for csh/tcsh in the login-shell fallback; single-line wrapper output is asserted byte-identical to the previous form./bin/cshand/bin/tcshfinds the same node candidates as under bash.src/main/ssh/suite passes (the two pre-existing failures —node:sqliteavailability and an ssh-config glob-order case — fail identically on a clean tree).🤖 Generated with Claude Code